Options in config file are currently disregarded when loading a model checkpoint, fixed this and also added the error message to cover cases when model architecture related parameters are mismatched between the checkpoint and config file.

This was to some degree intentional, so that a user did not have to know exactly the parameters that were used to initialize a model in order to load the weights. When was this causing an issue?

Options in config file are currently disregarded when loading a model checkpoint, fixed this and also added the error message to cover cases when model architecture related parameters are mismatched between the checkpoint and config file.

This was to some degree intentional, so that a user did not have to know exactly the parameters that were used to initialize a model in order to load the weights. When was this causing an issue?

Users might still want to modify non-architecture related parameters, like precursor_mass_tol in #245, and currently those aren't read from the config file. I think having the default config file reflect the parameters for the model for which there're released weights would be enough - as long as users don't change those they'd be good and they can still modify other parameters, they'd get an error message if they deviate from config defaults while using our weights.

I think having the default config file reflect the parameters for the model for which there're released weights would be enough - as long as users don't change those they'd be good and they can still modify other parameters, they'd get an error message if they deviate from config defaults while using our weights.

I don't think this is a very robust, particularly in the case when users may want to change models (for example, to a non-tryptic model). Are we always to going to re-train all of our submodels with an identical architecture to the main model?

Users might still want to modify non-architecture related parameters, like precursor_mass_tol in #245,

I think the better solution would be to explicitly support overwriting these parameters of model weights, maybe including a warning that the loaded weights do not match the parameter file.

I agree with Will that it would be better to use the parameters associated with the given model and to issue a warning to the user that the parameters in the provided config file do not match the given model (explicitly listing the given parameter value and the value that was used).

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e073415) 89.40% compared to head (8c88a71) 88.88%.
Report is 1 commits behind head on dev.

@@            Coverage Diff             @@
##              dev     #246      +/-   ##
==========================================
- Coverage   89.40%   88.88%   -0.52%     
==========================================
  Files          12       12              
  Lines         906      918      +12     
==========================================
+ Hits          810      816       +6     
- Misses         96      102       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I modified the PR to incorporate the changes @wfondrie suggested (thanks!) and added test cases.

Now we:

• Use checkpoint file for any mismatching model/architecture parameters (e.g. num_layers etc.) in checkpoint vs. config file and issue a warning about the specific mismatches.
• Use config file for any mismatching non-architecture related parameters (e.g. decoding, training etc.) in checkpoint vs. config file
• As before, we rely on the config file if any parameters are missing from the checkpoint and only raise an error if checkpoint is from an older incompatible release, i.e. has incompatible architectural components.